-
-
Notifications
You must be signed in to change notification settings - Fork 206
fix: Support character labels in assortativity_nominal()
#2432
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
Copilot
wants to merge
6
commits into
main
Choose a base branch
from
copilot/fix-assortativity-nominal-bug
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+403
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: krlmlr <[email protected]>
Co-authored-by: krlmlr <[email protected]>
Copilot
AI
changed the title
[WIP] Fix bug in assortativity_nominal for categorical labels
fix: Support character labels in Oct 26, 2025
assortativity_nominal()
Copilot finished work on behalf of
krlmlr
October 26, 2025 19:22
Contributor
|
@copilot: Only convert character vectors to integers, for clarity. |
Co-authored-by: krlmlr <[email protected]>
Contributor
Author
Copilot finished work on behalf of
krlmlr
October 26, 2025 20:44
krlmlr
requested changes
Oct 26, 2025
Contributor
krlmlr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot: Merge main, resolve conflicts as needed, and obey updated instructions.
Co-authored-by: krlmlr <[email protected]>
Copilot stopped work on behalf of
krlmlr due to an error
October 26, 2025 21:16
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix for assortativity_nominal with character types
assortativity_nominalfails with character labels (like 'A', 'B') becauseas.numeric()produces NAsas.integer(as.factor(types))only for character vectorsSummary
Fixed bug where
assortativity_nominal()would fail with character labels. The fix conditionally usesas.integer(as.factor(types))to convert character vectors to consecutive integers starting from 1, which is what the C implementation expects.Changes:
assortativity_nominal()inR/assortativity.Rto conditionally convert only character types usingas.integer(as.factor())Benefits:
Original prompt
This section details on the original issue you should resolve
<issue_title>bug in assortativity_nominal</issue_title>
<issue_description>Describe the bug
When trying to compute assortativity for categorical labels attached to nodes, an error is returned when the labels are letters but not when the labels are numbers. It is mentioned that the bug is unexpected and to report it with a reproducible example.
To reproduce
The script below reproduces the bug
Error in assortativity_nominal(g, types = V(g)$random3) :
At core/core/vector.pmt:126 : Assertion failed: size >= 0. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.
Please restart your R session to avoid crashes or other surprising behavior.
In addition: Warning message:
In assortativity_nominal(g, types = V(g)$random3) :
NAs introduced by coercion
Version information
Which version of igraph are you using and where did you obtain it?
igraph_1.6.0
from CRAN
R version 4.3.3 (2024-02-29)
Platform: x86_64-pc-linux-gnu (64-bit)
</issue_description>
Comments on the Issue (you are @copilot in this section)
@szhorvat > igraph_1.6.0Please always test with the latest version before reporting issues. I cannot reproduce it with 2.0.2. Can you try this version?</comment_new>
<comment_new>@szhorvat
With the current version of igraph, you will need to use consecutive integers, starting at 1, to represent categories. Names won't work.
I agree that the situation is not ideal. I must note that I am not an R user or R programmer, so I can't judge very well what is reasonable in R. Neither do I make the decision about whether we will do anything about this. But here's a suggestion for an improvement.
Let me know what you think @krlmlr
First, notice that the error message is not very good. It talks about negative indices, as in C we index the categories from 0. In R we index from 1. There's thus the usual problem about how to phrase the error to fit both. igraph/igraph#2119
The
typesargument here represents categorical data. It would indeed be very nice if other representations than indices could be supported, for example string names. Categorical data appears in many places in igraph as an input argument, such as:... and possibly others I'm not thinking of now.
Some high-level languages support categorical data directly. Isn't this what
factoris for in R? Mathematica does not have a data type for this, but I do have functions to convert other representations to category indices, and I allow categories to be specified in flexible ways.Categories also have different representations, each being most useful in specific contexts: we can assign a category name to each object/vertex: vertex 1 is "blue", vertex 2 is "red", vertex 3 is "blue"; or we can list the category members: "blue" contains {1, 3}, "red" contains {2}.
Should we then have a special Stimulus type specifically for categorical data? This would make it easy to auto-generate code that can handle various kinds of category representations that are convenient in the host language, and convert each to simple 0-based membership vectors that can be sent to C. The raw C errors we see here would never appear: error checking would be done by the function that converts the category representations. Users could work much more conveniently with such data.
Opinions, @krlmlr and @ntamas ?
Potentially related:
<comment_new>@ntamas
Yes, IMO it would be a good idea. Currently we have
VERTEX_COLORSandEDGE_COLORS(probably only in thedevelopbranch?). I think it's an ill-suited name but semantically it means the same thing, isn't it?</comment_new><comment_new>@krlmlr
A simple
types <- as.integer(as.factor(types))in `assortativity_nom...💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.